Fix SQLite sync retries and CLI actor loading - #4
Merged
Conversation
Keep SQLite caller bookkeeping and result observation inside the original synchronous deadline without retrying actor behavior. Load and register host app actors before CLI workers start, including when development eager loading is disabled.
Greptile SummaryThis PR extends the original synchronous deadline across SQLite coordination and result observation, while preserving caller-process state when database operations fail.
Confidence Score: 5/5The PR appears safe to merge, with no distinct actionable issue beyond the SQLite retry-loop concern already raised in the existing thread. The changed deadline propagation, process bookkeeping, final result probing, and CLI actor-loading paths preserve their documented contracts, and no new blocking failure remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[CLI boot] --> B[Load Rails environment]
B --> C[Eager-load managed app/actors directories]
C --> D[Register current actor classes]
D --> E[Start supervisor roles]
F[Synchronous actor call] --> G[Create original monotonic deadline]
G --> H[Enqueue durable message]
H --> I[Register or heartbeat caller process]
I --> J[Assist actor activation]
J --> K[Observe durable result]
K -->|completed| L[Return result]
K -->|deadline reached| M[Non-waiting final probe]
M -->|result committed| L
M -->|database still busy| N[Raise contention SyncTimeout]
Reviews (1): Last reviewed commit: "fix: bound sync retries and load actors" | Re-trigger Greptile |
cardmagic
marked this pull request as ready for review
August 7, 2026 16:56
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+87
to
+89
| def yield_before_retry | ||
| Thread.pass | ||
| end |
There was a problem hiding this comment.
Under sustained SQLite writer contention, every busy error immediately retries after only Thread.pass, repeatedly issuing database operations until the synchronous deadline and increasing CPU and lock contention compared with the removed bounded sleep.
Suggested change
| def yield_before_retry | |
| Thread.pass | |
| end | |
| def yield_before_retry | |
| sleep [ 0.001, SyncDeadline.remaining ].min | |
| end |
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/solid_objects/database_adapters/sqlite.rb
Line: 87-89
Comment:
**Hot SQLite retry loop**
Under sustained SQLite writer contention, every busy error immediately retries after only `Thread.pass`, repeatedly issuing database operations until the synchronous deadline and increasing CPU and lock contention compared with the removed bounded sleep.
```suggestion
def yield_before_retry
sleep [ 0.001, SyncDeadline.remaining ].min
end
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
There was a problem hiding this comment.
Pull request overview
This PR tightens SQLite synchronous-invocation behavior by extending deadline-bounded lock retry/probing to caller-process bookkeeping and result observation, and improves CLI correctness by ensuring host application actors are loaded/registered before workers start (including Rails dev reload scenarios). It also bumps the gem to 0.4.3 with corresponding docs, changelog, and generated RBS updates.
Changes:
- Extend SQLite sync coordination to use deadline-bounded
with_lock_retry/with_lock_probe, producing clearer timeout diagnostics under contention. - Preserve caller process registry state across retryable registration/heartbeat operations and cover more sync paths with retry/probe behavior.
- Load and (re)register application actors via a targeted Rails autoloader integration before CLI roles start; add integration tests and dummy-app scaffolding.
Reviewed changes
Copilot reviewed 23 out of 30 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| lib/solid_objects/synchronous_invocation.rb | Uses adapter lock retry/probe when loading messages and diagnosing timeouts at the sync deadline. |
| lib/solid_objects/sync_diagnostics.rb | Adds explicit database-contention timeout builders and refactors error construction/instrumentation. |
| lib/solid_objects/database_adapters/sqlite.rb | Introduces with_lock_retry / with_lock_probe to bound busy/lock handling by the original sync deadline. |
| lib/solid_objects/database_adapter.rb | Provides default no-op with_lock_retry / with_lock_probe hooks for non-SQLite adapters. |
| lib/solid_objects/client.rb | Wraps wait DB reads in lock retry under SyncDeadline and surfaces contention as SyncTimeout. |
| lib/solid_objects/process_registry.rb | Wraps process row create/update in lock retry and avoids losing in-memory state on retryable failures. |
| lib/solid_objects/caller_process.rb | Wraps process-record reuse checks in lock retry and preserves registry instance assignment ordering. |
| lib/solid_objects/activation.rb | Wraps instance lookup in lock retry and ensures ready-message yield updates run in a transaction. |
| lib/solid_objects/cli.rb | Installs the new application actor loader after booting the Rails environment. |
| lib/solid_objects/application_actor_loader.rb | New Rails/Zeitwerk-based loader to eager-load app/actors and register current actor classes; hooks into reload prepare callbacks. |
| lib/solid_objects/actor_registry.rb | Allows replacing a registered actor class when it appears to be a reload of the same named class. |
| lib/solid_objects/version.rb | Bumps version to 0.4.3. |
| test/unit/actor_registry_test.rb | Adds coverage for actor-class replacement on reload and rejection of distinct anonymous actor registrations. |
| test/integration/synchronous_invocation_test.rb | Adds SQLite contention tests covering caller-process registration/reuse/heartbeat and bounded sync timeouts. |
| test/integration/cli_test.rb | Adds an integration test ensuring CLI start loads application actors even when eager loading is disabled. |
| test/dummy/prepare_cli_worker.rb | New dummy-app helper to prepare schema/data for the CLI integration test. |
| test/dummy/config/initializers/cli_worker_probe.rb | New probe initializer to detect completion and terminate the CLI worker for the integration test. |
| test/dummy/config/database.yml | Allows dummy DB path override via SOLID_OBJECTS_DUMMY_DATABASE. |
| test/dummy/app/actors/cli_worker_actor.rb | New dummy actor used by the CLI integration test. |
| sig/generated/lib/solid_objects/synchronous_invocation.rbs | Updates signatures for new load/probe/diagnostic helpers. |
| sig/generated/lib/solid_objects/sync_diagnostics.rbs | Updates signatures for new contention helpers and refactored error builder. |
| sig/generated/lib/solid_objects/database_adapters/sqlite.rbs | Adds signatures for with_lock_retry, with_lock_probe, and retry-yield helper. |
| sig/generated/lib/solid_objects/database_adapter.rbs | Adds signatures for new adapter hook methods. |
| sig/generated/lib/solid_objects/application_actor_loader.rbs | Adds generated RBS for the new loader class. |
| sig/generated/lib/solid_objects/actor_registry.rbs | Adds signature for reload_of?. |
| README.md | Updates timeout/adapter behavior description and documents CLI actor loading behavior. |
| docs/operations.md | Documents CLI pre-loading of app/actors and reload-aware registration. |
| docs/correctness.md | Updates correctness/timeout narrative to include caller-process and SQLite contention reporting. |
| CHANGELOG.md | Adds 0.4.3 release notes. |
| Gemfile.lock | Updates locked gem version to 0.4.3. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+86
to
+89
| # @rbs () -> void | ||
| def yield_before_retry | ||
| Thread.pass | ||
| end |
Yield between deadline-aware lock attempts so a contended caller does not hammer SQLite while preserving the original sync deadline. Assert bounded retry pressure without re-executing actor behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Root causes
SQLite busy retries previously covered only
DatabaseAdapter#transaction. Caller registration, reuse, heartbeat, and some synchronous result observations ran outside that boundary, allowing rawActiveRecord::StatementTimeoutexceptions to escape after a message had committed.The CLI required the Rails environment, but development eager loading remained disabled. A standalone worker therefore saw only persisted actor type strings and could claim a message before its actor class had registered.
Correctness
Retries cover short database-only coordination operations and never wrap actor handlers, rendering, lifecycle hooks, or external I/O. They use the original monotonic deadline and a non-waiting final diagnostic probe. Actor execution remains protected by the existing lease and fencing rules.
The CLI targets only Rails-managed
app/actorsdirectories through the main Zeitwerk loader. It registers inferred actor types and installs a preparation callback so reloaded classes replace their prior registry entry. Unknown actor types still fail visibly.No database migration is required.
Validation
bundle exec rake: 216 tests, 845 assertions, zero failures; Standard, RuboCop, RBS validation, Steep, and Brakeman passedgem build solid_objects.gemspec --output /tmp/solid_objects-0.4.3.gemgit diff --check